feat(benchmark): per-run output paths and cluster membership - #24
Conversation
Every benchmark run wrote data/benchmark-results.json, so running a second comparison silently destroyed the first one's numbers. That already cost a local nomic-vs-embeddinggemma run: the hosted comparison started afterwards and overwrote it, leaving only console scrollback. `--out <path>` gives a run its own file. An empty or directory-shaped value throws instead of falling back to the default, since that fallback is exactly the overwrite being avoided. Parent directories are created. Results now also record `clusterMembership` per model per threshold. Cluster counts alone cannot answer the question the benchmark exists to answer: a model that finds more clusters is either catching duplicates the incumbent misses or chaining unrelated items, and the counts look identical either way. Membership can only otherwise be recovered by re-embedding the whole corpus. Also drops the stale caveat on starmap `createdAt` now that star-map's importer patch is open as PR #11.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6550e8768e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| items: c.items.map((item) => item.number), | ||
| })); | ||
| baseStore.close(); | ||
| clusterMembership.push({ model: baseline.model, threshold: t, clusters: baseSimplified }); |
There was a problem hiding this comment.
When a model database already exists from an earlier run and the repository's fetched corpus has since changed, getAllItems(repoFull) includes rows that were not fetched in this run because runBenchmarkForModel only upserts and never removes them. This newly serialized membership can therefore list deleted or page-limit-evicted items and disagree with clustersByThreshold, which was computed using the current allItems; clear obsolete rows or restrict membership clustering to IDs from the current fetch.
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| const outPath = resolve(process.cwd(), "data", "benchmark-results.json"); | ||
| const outPath = resolveBenchmarkOutPath(opts.out); |
There was a problem hiding this comment.
When --out is empty, directory-shaped, or otherwise invalid, this validation is not reached until after every model has been embedded and all clustering has completed. A multi-hour run then fails without writing its JSON—the exact data loss this option is intended to prevent—so resolve and syntactically validate the path before fetching or embedding begins.
Useful? React with 👍 / 👎.
1 participant
Every benchmark run wrote
data/benchmark-results.json. Two runs in a row therefore left only the second one's numbers on disk.That already cost a run: a local nomic-vs-embeddinggemma comparison was overwritten by a hosted one started afterwards, leaving only console scrollback. Ten minutes of embedding, gone to starting the next comparison.
--out <path>Gives a run its own file. Parent directories are created. An empty or directory-shaped value throws rather than falling back to the default, since that fallback is exactly the overwrite being avoided.
clusterMembershipResults now record which items landed in which cluster, per model per threshold.
Cluster counts cannot answer the question the benchmark exists to answer. A model that finds more clusters is either catching duplicates the incumbent misses or chaining unrelated items together, and the counts look identical in both cases. Membership is otherwise only recoverable by re-embedding the whole corpus.
It is what settled the question for
embeddinggemma: of the 13 clusters it found that nomic missed, 11 were real duplicates and 2 were recurring dependabot PRs. That is unreadable from13.440 tests, lint and typecheck clean.